Skip to content

Update j-jobs to be bash linter compliant #4124

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 4 commits into from
Oct 6, 2025
Merged

Update j-jobs to be bash linter compliant #4124

merged 4 commits into from
Oct 6, 2025

Conversation

@WalterKolczynski-NOAA
Copy link
Contributor

@WalterKolczynski-NOAA WalterKolczynski-NOAA commented Oct 2, 2025 •
edited
Loading

Description

Updates the j-job and pre-jjob bash scripts to be compliant with both shellcheck and shfmt. This does not enforce some other standards not checked by shellcheck.

The .editorconfig is also updated with guidelines for formatting bash files (used by shfmt). Two different blocks with identical settings had to be used to accommodate j-jobs, which have no file suffix.

Refs: #397

Type of change

  • Bug fix (fixes something broken)
  • New feature (adds functionality)
  • Maintenance (code refactor, clean-up, new CI test, etc.)

Change characteristics

  • Is this a breaking change (a change in existing functionality)? YES/NO
  • Does this change require a documentation update? YES/NO
  • Does this change require an update to any of the following submodules? NO

How has this been tested?

  • All tests on Ursa
  • Following check after changes to ensure compliance:
20:53:06 <feature/shfmt_jjobs> shfmt_jjobs/$shellcheck jobs/* dev/jobs/*
20:53:18 <feature/shfmt_jjobs> shfmt_jjobs/$shfmt -d jobs dev/jobs
20:53:27 <feature/shfmt_jjobs> shfmt_jjobs/$

Checklist

  • Any dependent changes have been merged and published
  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have documented my code, including function, input, and output descriptions
  • My changes generate no new warnings
  • New and existing tests pass with my changes
  • This change is covered by an existing CI test or a new one has been added
  • Any new scripts have been added to the .github/CODEOWNERS file with owners
  • I have made corresponding changes to the system documentation if necessary

@WalterKolczynski-NOAA
Bring non-dev j-jobs into shellcheck compliance
5310374 
Updated all non-dev j-job scripts to be shellcheck compliant.
@WalterKolczynski-NOAA
Update j-jobs with shfmt
8f45c25
AndrewEichmann-NOAA
AndrewEichmann-NOAA previously approved these changes Oct 3, 2025
View reviewed changes
Copy link
Contributor

@AndrewEichmann-NOAA AndrewEichmann-NOAA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @WalterKolczynski-NOAA !
Eyeballed marine DA scripts (jobs/JGLOBAL_MARINE* and jobs/JGLOBAL_PREP_OCEAN_OBS), looks good

JessicaMeixner-NOAA
JessicaMeixner-NOAA previously approved these changes Oct 3, 2025
View reviewed changes
Copy link
Contributor

@JessicaMeixner-NOAA JessicaMeixner-NOAA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wave job changes were white spaces or adding quotes. As long as tests pass, I approve.

guillaumevernieres
guillaumevernieres previously approved these changes Oct 3, 2025
View reviewed changes
Copy link
Contributor

@guillaumevernieres guillaumevernieres left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

DavidNew-NOAA
DavidNew-NOAA previously approved these changes Oct 3, 2025
View reviewed changes
DavidHuber-NOAA
DavidHuber-NOAA reviewed Oct 3, 2025
View reviewed changes
bbakernoaa
bbakernoaa previously approved these changes Oct 3, 2025
View reviewed changes
BoCui-NOAA
BoCui-NOAA previously approved these changes Oct 3, 2025
View reviewed changes
WenMeng-NOAA
WenMeng-NOAA previously approved these changes Oct 3, 2025
View reviewed changes
Copy link
Contributor

@WenMeng-NOAA WenMeng-NOAA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The updates related to UPP and ATMOS Product jobs look good to me.

DavidHuber-NOAA
DavidHuber-NOAA reviewed Oct 3, 2025
View reviewed changes
Copy link
Contributor

@DavidHuber-NOAA DavidHuber-NOAA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Just some suggestions on further cleanup.

EdwardSafford-NOAA
EdwardSafford-NOAA previously approved these changes Oct 3, 2025
View reviewed changes
Copy link
Contributor

@EdwardSafford-NOAA EdwardSafford-NOAA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Monitor changes look good to me.

DavidHuber-NOAA
DavidHuber-NOAA approved these changes Oct 6, 2025
View reviewed changes
Copy link
Contributor

@DavidHuber-NOAA DavidHuber-NOAA left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @WalterKolczynski-NOAA for going the extra mile!

@DavidHuber-NOAA
Copy link
Contributor

Launching tests on WCOSS2.

@DavidHuber-NOAA DavidHuber-NOAA added CI-Wcoss2-Ready PR is ready for CI testing on WCOSS2. CI-Wcoss2-Building CI testing is cloning/building on WCOSS2 and removed CI-Wcoss2-Ready PR is ready for CI testing on WCOSS2. labels Oct 6, 2025
@DavidHuber-NOAA
Copy link
Contributor

Tests set up to run in /lfs/h2/emc/stmp/david.huber/RT_4124

@DavidHuber-NOAA DavidHuber-NOAA added CI-Wcoss2-Running CI testing on WCOSS for this PR is in-progress and removed CI-Wcoss2-Building CI testing is cloning/building on WCOSS2 labels Oct 6, 2025
@DavidHuber-NOAA
Copy link
Contributor

All tests passed on WCOSS2. Merging.

@DavidHuber-NOAA DavidHuber-NOAA added CI-Wcoss2-Passed CI testing on WCOSS for this PR has completed successfully and removed CI-Wcoss2-Running CI testing on WCOSS for this PR is in-progress labels Oct 6, 2025
@DavidHuber-NOAA DavidHuber-NOAA merged commit bff49fd into NOAA-EMC:develop Oct 6, 2025
5 checks passed
@WalterKolczynski-NOAA WalterKolczynski-NOAA deleted the feature/shfmt_jjobs branch October 7, 2025 00:52
weihuang-jedi added a commit to NOAA-EPIC/global-workflow-cloud that referenced this pull request Oct 8, 2025
@weihuang-jedi
Merge branch 'develop' of github.com:NOAA-EPIC/global-workflow-cloud …
958ae08 
…into feature/adjust_tasks_per_node_layout

* 'develop' of github.com:NOAA-EPIC/global-workflow-cloud:
  Update CODEOWNERS for new oceanice_products assignments (NOAA-EMC#4134)
  Improve GW forecast timing of high-resolution runs with findings from containerized GW (NOAA-EMC#4123)
  Update CODEOWNERS to change job assignments (NOAA-EMC#4128)
  Update j-jobs to be bash linter compliant (NOAA-EMC#4124)
  Update the marine analysis archiving (NOAA-EMC#4082)
  Port tracker/genesis tasks to cloud; load wgrib2; add noaacloud to load_gw_gsi_modules (NOAA-EMC#4111)
  Only run wave bnd pnt for gfs (NOAA-EMC#4112)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@DavidHuber-NOAA DavidHuber-NOAA DavidHuber-NOAA approved these changes

@guillaumevernieres guillaumevernieres guillaumevernieres left review comments

@bbakernoaa bbakernoaa bbakernoaa left review comments

@JessicaMeixner-NOAA JessicaMeixner-NOAA JessicaMeixner-NOAA left review comments

@jiaruidong2017 jiaruidong2017 jiaruidong2017 left review comments

@WenMeng-NOAA WenMeng-NOAA WenMeng-NOAA left review comments

@BoCui-NOAA BoCui-NOAA BoCui-NOAA left review comments

@AndrewEichmann-NOAA AndrewEichmann-NOAA AndrewEichmann-NOAA left review comments

@EdwardSafford-NOAA EdwardSafford-NOAA EdwardSafford-NOAA left review comments

@DavidNew-NOAA DavidNew-NOAA DavidNew-NOAA left review comments

@sbanihash sbanihash Awaiting requested review from sbanihash sbanihash is a code owner

@aerorahul aerorahul Awaiting requested review from aerorahul aerorahul is a code owner

@CoryMartin-NOAA CoryMartin-NOAA Awaiting requested review from CoryMartin-NOAA CoryMartin-NOAA is a code owner

@GwenChen-NOAA GwenChen-NOAA Awaiting requested review from GwenChen-NOAA

@ChristopherHill-NOAA ChristopherHill-NOAA Awaiting requested review from ChristopherHill-NOAA ChristopherHill-NOAA is a code owner

@EricSinsky-NOAA EricSinsky-NOAA Awaiting requested review from EricSinsky-NOAA EricSinsky-NOAA is a code owner

@RussTreadon-NOAA RussTreadon-NOAA Awaiting requested review from RussTreadon-NOAA RussTreadon-NOAA is a code owner

@CatherineThomas-NOAA CatherineThomas-NOAA Awaiting requested review from CatherineThomas-NOAA CatherineThomas-NOAA is a code owner

@JiayiPeng-NOAA JiayiPeng-NOAA Awaiting requested review from JiayiPeng-NOAA JiayiPeng-NOAA is a code owner

@jack-woollen jack-woollen Awaiting requested review from jack-woollen jack-woollen is a code owner

@TerrenceMcGuinness-NOAA TerrenceMcGuinness-NOAA Awaiting requested review from TerrenceMcGuinness-NOAA

Assignees

No one assigned

Labels

CI-Wcoss2-Passed CI testing on WCOSS for this PR has completed successfully

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.